Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): add payments post_session_tokens flow #6202

Merged
merged 38 commits into from
Oct 15, 2024
Merged

Conversation

swangi-kumari
Copy link
Contributor

@swangi-kumari swangi-kumari commented Oct 2, 2024

Type of Change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates
  • Documentation
  • CI/CD

Description

  1. Add support for a new endpoint post_session_tokens which can be called before the confirm call.

Use Case

  • In PayPal, after the session call, when we click the PayPal button, we need to create an order. Previously, this was done in the /confirm call, and in response, we received the order_id / connector_transaction_id. We would then send this information to the HS SDK, which forwarded it to PayPal. PayPal used this data to render their SDK, allowing the user to log in and modify the shipping address. The user would then click "Complete Purchase," triggering the OnApprove callback, which was handled by the /complete_authorize call.
  • The /confirm call happens at the point of order creation, but we receive the shipping address after that. At that stage, we make a call to TaxJar, and the order's tax amount is added. However, since the /confirm call has already happened, we can't change/update the amount at that point. This was the reason for creating the new flow, where the order creation happens in the /post_session_tokens call, and the /confirm call occurs after tax calculation.
  • After this refactoring, the CreateOrder step will be handled in the /post_session_tokens call, and the OnApprove action will be handled in the /confirm call.
  1. Populate Dynamic Fields for Paypal

Additional Changes

  • This PR modifies the API contract
  • This PR modifies the database schema
  • This PR modifies application configuration/environment variables

Motivation and Context

How did you test it?

  1. Create Merchant Account
curl --location 'http://localhost:8080/accounts' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'api-key: test_admin' \
--data-raw '{
  "merchant_id": "merchant_1728546485",
  "locker_id": "m0010",
  "merchant_name": "NewAge Retailer",
  "merchant_details": {
    "primary_contact_person": "John Test",
    "primary_email": "JohnTest@test.com",
    "primary_phone": "sunt laborum",
    "secondary_contact_person": "John Test2",
    "secondary_email": "JohnTest2@test.com",
    "secondary_phone": "cillum do dolor id",
    "website": "https://www.example.com",
    "about_business": "Online Retail with a wide selection of organic products for North America",
    "address": {
      "line1": "1467",
      "line2": "Harrison Street",
      "line3": "Harrison Street",
      "city": "San Fransico",
      "state": "California",
      "zip": "94122",
      "country": "US"
    }
  },
  "return_url": "https://google.com/success",
  "webhook_details": {
    "webhook_version": "1.0.1",
    "webhook_username": "ekart_retail",
    "webhook_password": "password_ekart@123",
    "payment_created_enabled": true,
    "payment_succeeded_enabled": true,
    "payment_failed_enabled": true
  },
  "sub_merchants_enabled": false,
  "metadata": {
    "city": "NY",
    "unit": "245"
  },
  "primary_business_details": [
    {
      "country": "US",
      "business": "food"
    }
  ]
}'
  1. Create API Key
curl --location 'http://localhost:8080/api_keys/merchant_1728540504' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'api-key: test_admin' \
--data '{
  "name": "API Key 1",
  "description": null,
  "expiration": "2025-09-23T01:02:03.000Z"
}'
  1. Create MCA
curl --location 'http://localhost:8080/account/merchant_1728540504/connectors?=' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'api-key: test_admin' \
--data '{
    "connector_type": "payment_processor",
    "connector_name": "paypal",
    "connector_account_details": {
        "auth_type": "BodyKey",
        "api_key": "_",
        "key1": "_"
    },
    "test_mode": false,
    "disabled": false,
    "payment_methods_enabled": [
        {
            "payment_method": "card",
            "payment_method_types": [
                {
                    "payment_method_type": "credit",
                    "card_networks": [
                        "Visa",
                        "Mastercard"
                    ],
                    "minimum_amount": 1,
                    "maximum_amount": 68607706,
                    "recurring_enabled": true,
                    "installment_payment_enabled": true
                },
                {
                    "payment_method_type": "debit",
                    "card_networks": [
                        "Visa",
                        "Mastercard"
                    ],
                    "minimum_amount": 1,
                    "maximum_amount": 68607706,
                    "recurring_enabled": true,
                    "installment_payment_enabled": true
                }
            ]
        },
        {
            "payment_method": "wallet",
            "payment_method_types": [
                {
                    "payment_method_type": "paypal",
                    "payment_experience": "redirect_to_url",
                    "minimum_amount": 1,
                    "maximum_amount": 68607706,
                    "recurring_enabled": false,
                    "installment_payment_enabled": false
                },
                {
                    "payment_method_type": "paypal",
                    "payment_experience": "invoke_sdk_client",
                    "minimum_amount": 1,
                    "maximum_amount": 68607706,
                    "recurring_enabled": false,
                    "installment_payment_enabled": false
                }
                
            ]
        },
        {
            "payment_method": "bank_redirect",
            "payment_method_types": [
                {
                    "payment_method_type": "giropay",
                    "recurring_enabled": true,
                    "installment_payment_enabled": true,
                    "payment_experience": "redirect_to_url"
                },
                {
                    "payment_method_type": "ideal",
                    "recurring_enabled": true,
                    "installment_payment_enabled": true,
                    "payment_experience": "redirect_to_url"
                },
                {
                    "payment_method_type": "eps",
                    "recurring_enabled": true,
                    "installment_payment_enabled": true,
                    "payment_experience": "redirect_to_url"
                },
                {
                    "payment_method_type": "sofort",
                    "recurring_enabled": true,
                    "installment_payment_enabled": true,
                    "payment_experience": "redirect_to_url"
                }
            ]
        }
    ],
     "metadata": {
        "paypal_sdk": {
            "client_id": "___"
        }
    },
    "business_country": "US",
    "business_label": "food"
}'
  1. Create Payment
curl --location 'http://localhost:8080/payments' \
--header 'Content-Type: application/json' \
--header 'Accept: application/json' \
--header 'api-key: dev_MTxKOCiP8GOiXv955xYCMuW0MUzl1anEbVVkxBBR87wfDUObqYqOsCqpogsFFnCd' \
--data-raw '{
    "amount": 6540,
    "currency": "USD",
    "confirm": false,
    "capture_method": "automatic",
    "capture_on": "2022-09-10T10:11:12Z",
    "amount_to_capture": 6540,
    "customer_id": "StripeCustomer",
    "email": "guest@example.com",
    "name": "John Doe",
    "phone": "999999999",
    "phone_country_code": "+1",
    "description": "Its my first payment request",
    "authentication_type": "no_three_ds",
    "return_url": "https://duck.com",
    "billing": {
        "address": {
            "line1": "1467",
            "line2": "Harrison Street",
            "line3": "Harrison Street",
            "city": "San Fransico",
            "state": "California",
            "zip": "94122",
            "country": "US",
            "first_name": "PiX"
        }
    },
    "shipping": {
        "address": {
            "line1": "1467",
            "line2": "Harrison Street",
            "line3": "Harrison Street",
            "city": "San Fransico",
            "state": "California",
            "zip": "94122",
            "country": "US",
            "first_name": "PiX"
        }
    },
    "statement_descriptor_name": "joseph",
    "statement_descriptor_suffix": "JS",
    "metadata": {
        "udf1": "value1",
        "new_customer": "true",
        "login_date": "2019-09-10T10:11:12Z"
    }
}'
  1. Create Session token
curl --location 'http://localhost:8080/payments/session_tokens' \
--header 'Content-Type: application/json' \
--header 'api-key: pk_dev_10cfee22267a4184a3d45665920788ea' \
--data '{
    "payment_id": "pay_J3IXGvol8nNvUkSRTcwQ",
    "wallets": [],
    "client_secret": "pay_J3IXGvol8nNvUkSRTcwQ_secret_VN76rK4oCqTghQ1kwNcO"
}'

Response

{
    "payment_id": "pay_mcVzUMZws5KmEKbGkZ0g",
    "client_secret": "pay_mcVzUMZws5KmEKbGkZ0g_secret_RMWlmy1jSdlp9VOMX1qc",
    "session_token": [
        {
            "wallet_name": "paypal",
            "connector": "paypal",
            "session_token": "ASKAGh2WXgqfQ5TzjpZzLsfhVGlFbjq5VrV5IOX8KXDD2N_XqkGeYNDkWyr_UXnfhXpEkABdmP284b_2",
            "sdk_next_action": {
                "next_action": "post_session_tokens"
            }
        }
    ]
}

In Response Check for Next action - It should be post_session_tokens

  1. List Payment Methods for merchant - Check for required fields (shipping_details)
curl --location 'http://localhost:8080/account/payment_methods?client_secret=pay_J3IXGvol8nNvUkSRTcwQ_secret_VN76rK4oCqTghQ1kwNcO' \
--header 'Accept: application/json' \
--header 'api-key: pk_dev_10cfee22267a4184a3d45665920788ea'

Response

{
    "redirect_url": "https://google.com/success",
    "currency": "USD",
    "payment_methods": [
        {
            "payment_method": "wallet",
            "payment_method_types": [
                {
                    "payment_method_type": "paypal",
                    "payment_experience": [
                        {
                            "payment_experience_type": "invoke_sdk_client",
                            "eligible_connectors": [
                                "paypal"
                            ]
                        },
                        {
                            "payment_experience_type": "redirect_to_url",
                            "eligible_connectors": [
                                "paypal"
                            ]
                        }
                    ],
                    "card_networks": null,
                    "bank_names": null,
                    "bank_debits": null,
                    "bank_transfers": null,
                    "required_fields": {
                        "shipping.address.country": {
                            "required_field": "shipping.address.country",
                            "display_name": "country",
                            "field_type": {
                                "user_shipping_address_country": {
                                    "options": [
                                        "ALL"
                                    ]
                                }
                            },
                            "value": "US"
                        },
                        "shipping.address.first_name": {
                            "required_field": "shipping.address.first_name",
                            "display_name": "shipping_first_name",
                            "field_type": "user_shipping_name",
                            "value": "PiX"
                        },
                        "shipping.address.zip": {
                            "required_field": "shipping.address.zip",
                            "display_name": "zip",
                            "field_type": "user_shipping_address_pincode",
                            "value": "94122"
                        },
                        "shipping.address.state": {
                            "required_field": "shipping.address.state",
                            "display_name": "state",
                            "field_type": "user_shipping_address_state",
                            "value": "California"
                        },
                        "shipping.address.city": {
                            "required_field": "shipping.address.city",
                            "display_name": "city",
                            "field_type": "user_shipping_address_city",
                            "value": "San Fransico"
                        },
                        "shipping.address.last_name": {
                            "required_field": "shipping.address.last_name",
                            "display_name": "shipping_last_name",
                            "field_type": "user_shipping_name",
                            "value": null
                        },
                        "shipping.address.line1": {
                            "required_field": "shipping.address.line1",
                            "display_name": "line1",
                            "field_type": "user_shipping_address_line1",
                            "value": "1467"
                        }
                    },
                    "surcharge_details": null,
                    "pm_auth_connector": null
                }
            ]
        },
            ],
    "mandate_payment": null,
    "merchant_name": "NewAge Retailer",
    "show_surcharge_breakup_screen": false,
    "payment_type": "normal",
    "request_external_three_ds_authentication": false,
    "collect_shipping_details_from_wallets": false,
    "collect_billing_details_from_wallets": false,
    "is_tax_calculation_enabled": false
}
  1. Create Post Session Tokens for Paypal
curl --location 'http://localhost:8080/payments/pay_J3IXGvol8nNvUkSRTcwQ/post_session_tokens' \
--header 'Content-Type: application/json' \
--header 'api-key: pk_dev_10cfee22267a4184a3d45665920788ea' \
--data '{
    "client_secret": "pay_J3IXGvol8nNvUkSRTcwQ_secret_VN76rK4oCqTghQ1kwNcO",
    "payment_method_type": "paypal",
    "payment_method": "wallet"
}'

Response

 {
    "payment_id": "pay_mcVzUMZws5KmEKbGkZ0g",
    "next_action": {
        "type": "invoke_sdk_client",
        "next_action_data": {
            "next_action": "confirm",
            "order_id": "2KW93090954330301"
        }
    }
}

In Response Check for Next action - It should be confirm

  1. Confirm Request
curl 'http://localhost:8080/payments/pay_6oSZ2utmkjgdOlfrZ53E/confirm' \
-X 'POST' \
-H 'Content-Type: application/json' \
-H 'Accept-Encoding: gzip, deflate' \
-H 'Sec-Fetch-Mode: cors' \
-H 'x-payment-confirm-source: sdk' \
-H 'x-client-source: ExpressCheckoutElement' \
-H 'api-key: pk_dev_43922dfcb2694e09929891b0f4f7b6dc' \
-H 'x-browser-version: 605.1.15' \
--data-binary '{"client_secret":"pay_6oSZ2utmkjgdOlfrZ53E_secret_EFrF7Izc98NHygNyG51x","return_url":"http://localhost:9060","retry_action":"manual_retry","shipping":{"address":{"country":"US","state":"California","zip":"94588","first_name":"Jagan","city":"Pleasanton","last_name":"Elavarasan","line1":"4141"}},"payment_method":"wallet","payment_method_type":"paypal","payment_experience":"invoke_sdk_client","connector":["paypal"],"payment_method_data":{"wallet":{"paypal_sdk":{"token":""}}},"browser_info":{"user_agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.1.2 Safari/605.1.15","accept_header":"text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8","language":"en-US","color_depth":24,"screen_height":1117,"screen_width":1728,"time_zone":-330,"java_enabled":true,"java_script_enabled":true},"payment_type":"normal"}'

Response

    "payment_id": "pay_6oSZ2utmkjgdOlfrZ53E",
    "merchant_id": "merchant_1728844379",
    "status": "succeeded",
    "amount": 2999,
    "net_amount": 2999,
    "amount_capturable": 0,
    "amount_received": 2999,
    "connector": "paypal",
    "client_secret": "pay_6oSZ2utmkjgdOlfrZ53E_secret_EFrF7Izc98NHygNyG51x",
    "created": "2024-10-14T09:09:36.340Z",
    "currency": "USD",
    "customer_id": "hyperswitch_sdk_demo_id",
    "customer": {
        "id": "hyperswitch_sdk_demo_id",
        "name": null,
        "email": "hyperswitch_sdk_demo_id@gmail.com",
        "phone": null,
        "phone_country_code": null
    },
    "description": "Hello this is description",
    "refunds": null,
    "disputes": null,
    "mandate_id": null,
    "mandate_data": null,
    "setup_future_usage": null,
    "off_session": null,
    "capture_on": null,
    "capture_method": "automatic",
    "payment_method": "wallet",
    "payment_method_data": {
        "wallet": {},
        "billing": null
    },
    "payment_token": null,
    "shipping": {
        "address": {
            "city": "Pleasanton",
            "country": "US",
            "line1": "4141",
            "line2": "Amsterdam Ave",
            "line3": "alsksoe",
            "zip": "94588",
            "state": "California",
            "first_name": "Jagan",
            "last_name": "Elavarasan"
        },
        "phone": {
            "number": "5551234",
            "country_code": "+1"
        },
        "email": "customer@email.us"
    },
    "billing": {
        "address": {
            "city": "San Fransico",
            "country": "US",
            "line1": "1467",
            "line2": "Harrison Street",
            "line3": "Harrison Street",
            "zip": "94122",
            "state": "California",
            "first_name": "joseph",
            "last_name": "Doe"
        },
        "phone": {
            "number": "_",
            "country_code": "+91"
        },
        "email": "swangi@getMaxListeners.com"
    },
    "order_details": [
        {
            "brand": null,
            "amount": 2999,
            "category": null,
            "quantity": 1,
            "product_id": null,
            "product_name": "Apple iPhone 15",
            "product_type": null,
            "sub_category": null,
            "product_img_link": null,
            "product_tax_code": null,
            "requires_shipping": null
        }
    ],
    "email": "hyperswitch_sdk_demo_id@gmail.com",
    "name": null,
    "phone": null,
    "return_url": "http://localhost:9060/",
    "authentication_type": "three_ds",
    "statement_descriptor_name": null,
    "statement_descriptor_suffix": null,
    "next_action": null,
    "cancellation_reason": null,
    "error_code": null,
    "error_message": null,
    "unified_code": null,
    "unified_message": null,
    "payment_experience": "invoke_sdk_client",
    "payment_method_type": "paypal",
    "connector_label": null,
    "business_country": null,
    "business_label": "default",
    "business_sub_label": null,
    "allowed_payment_method_types": null,
    "ephemeral_key": null,
    "manual_retry_allowed": false,
    "connector_transaction_id": "1SV0561343891340C",
    "frm_message": null,
    "metadata": {
        "udf1": "value1",
        "login_date": "2019-09-10T10:11:12Z",
        "new_customer": "true"
    },
    "connector_metadata": null,
    "feature_metadata": null,
    "reference_id": "pay_6oSZ2utmkjgdOlfrZ53E_1",
    "payment_link": null,
    "profile_id": "pro_03iBylVoOpuldQsVZKtO",
    "surcharge_details": null,
    "attempt_count": 1,
    "merchant_decision": null,
    "merchant_connector_id": "mca_frKbF7wxB3Rt7tAFbpJA",
    "incremental_authorization_allowed": null,
    "authorization_count": null,
    "incremental_authorizations": null,
    "external_authentication_details": null,
    "external_3ds_authentication_attempted": false,
    "expires_on": "2024-10-14T09:24:36.340Z",
    "fingerprint": null,
    "browser_info": {
        "language": "en-US",
        "time_zone": -330,
        "ip_address": "::1",
        "user_agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.1.2 Safari/605.1.15",
        "color_depth": 24,
        "java_enabled": true,
        "screen_width": 1728,
        "accept_header": "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8",
        "screen_height": 1117,
        "java_script_enabled": true
    },
    "payment_method_id": null,
    "payment_method_status": null,
    "updated": "2024-10-14T09:10:19.289Z",
    "charges": null,
    "frm_metadata": null,
    "merchant_order_reference_id": null,
    "order_tax_amount": null,
    "connector_mandate_id": null
}

For end to end testing, we need to do the payment via SDK

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed the submitted code
  • I added unit tests for my changes where possible

Copy link

semanticdiff-com bot commented Oct 2, 2024

Review changes with SemanticDiff.

Analyzed 40 of 41 files.

Overall, the semantic diff is 8% smaller than the GitHub diff.

File Information
Filename Status
✔️ crates/router_env/src/logger/types.rs Analyzed
✔️ crates/router_derive/src/macros/operation.rs Analyzed
✔️ crates/router/src/types.rs 78.0% smaller
✔️ crates/router/src/types/api/payments.rs 90.74% smaller
✔️ crates/router/src/types/api/payments_v2.rs 89.53% smaller
✔️ crates/router/src/services/api.rs Analyzed
✔️ crates/router/src/services/authentication.rs Analyzed
✔️ crates/router/src/routes/app.rs 21.33% smaller
✔️ crates/router/src/routes/lock_utils.rs 82.81% smaller
✔️ crates/router/src/routes/payments.rs Analyzed
✔️ crates/router/src/core/payments.rs 47.84% smaller
✔️ crates/router/src/core/payments/connector_integration_v2_impls.rs Analyzed
✔️ crates/router/src/core/payments/flows.rs Analyzed
✔️ crates/router/src/core/payments/operations.rs 62.76% smaller
✔️ crates/router/src/core/payments/transformers.rs Analyzed
✔️ crates/router/src/core/payments/operations/payment_post_session_tokens.rs Analyzed
✔️ crates/router/src/core/payments/operations/payment_response.rs 9.49% smaller
✔️ crates/router/src/core/payments/flows/post_session_tokens_flow.rs Analyzed
✔️ crates/router/src/core/payments/flows/session_flow.rs Analyzed
✔️ crates/router/src/connector/paypal.rs 15.33% smaller
✔️ crates/router/src/connector/utils.rs Analyzed
✔️ crates/router/src/connector/paypal/transformers.rs 11.38% smaller
✔️ crates/router/src/configs/defaults.rs 8.66% smaller
✔️ crates/openapi/src/openapi.rs Analyzed
✔️ crates/openapi/src/routes/payments.rs Analyzed
✔️ crates/hyperswitch_interfaces/src/types.rs 42.98% smaller
✔️ crates/hyperswitch_interfaces/src/api/payments.rs 35.97% smaller
✔️ crates/hyperswitch_interfaces/src/api/payments_v2.rs 24.12% smaller
✔️ crates/hyperswitch_domain_models/src/router_request_types.rs Analyzed
✔️ crates/hyperswitch_domain_models/src/types.rs 66.25% smaller
✔️ crates/hyperswitch_domain_models/src/router_flow_types/payments.rs Analyzed
✔️ crates/hyperswitch_domain_models/src/payments/payment_attempt.rs Analyzed
✔️ crates/hyperswitch_connectors/src/default_implementations.rs 7.2% smaller
✔️ crates/hyperswitch_connectors/src/default_implementations_v2.rs 32.11% smaller
✔️ crates/diesel_models/src/payment_attempt.rs Analyzed
✔️ crates/api_models/src/payments.rs Analyzed
✔️ crates/api_models/src/events/payment.rs 13.61% smaller
✔️ api-reference-v2/openapi_spec.json Analyzed
✔️ api-reference/mint.json 66.67% smaller
✔️ api-reference/openapi_spec.json Analyzed
api-reference/api-reference/payments/payments--post-session-tokens.mdx Unsupported file format

@hyperswitch-bot hyperswitch-bot bot added the M-api-contract-changes Metadata: This PR involves API contract changes label Oct 3, 2024
@swangi-kumari swangi-kumari self-assigned this Oct 4, 2024
@swangi-kumari swangi-kumari added A-connector-integration Area: Connector integration A-core Area: Core flows C-feature Category: Feature request or enhancement C-refactor Category: Refactor labels Oct 8, 2024
@swangi-kumari swangi-kumari changed the title feat(core): add payments post_session_update flow feat(core): add payments post_session_tokens flow Oct 14, 2024
}

#[derive(Debug, serde::Deserialize, serde::Serialize, Clone, ToSchema)]
pub struct PaymentsPostSessionTokensResponse {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we using this response anywhere? I think we are sending the payments response. Instead of that can we use this struct?

)
.await?;

let payment_method_billing = helpers::get_address_by_id(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will not have this billing address in this flow, can be removed

Copy link
Contributor

@sai-harsha-vardhan sai-harsha-vardhan Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Billing address would be there if merchant has sent it in payments create request right? As, this is create order call and connector expects billing address, we should be sending it? @Narayanbhat166

let m_db = db.clone().store;
let payment_attempt_update =
storage::PaymentAttemptUpdate::PostSessionTokensUpdate {
connector_transaction_id: connector_transaction_id.clone(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not update connector_transaction_id in this flow. There can be cases where confirm happens through a different payment method data and a new connector might be selected, we will have the wrong mapping of connector and transaction id.

connector: payment_data
.payment_attempt
.connector
.clone()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we will not store the connector in this case. We should not update the error message in this case. This should be a bad request error.

@@ -475,6 +475,11 @@ pub enum PaymentAttemptUpdate {
unified_message: Option<String>,
connector_transaction_id: Option<String>,
},
PostSessionTokensUpdate {
updated_by: String,
connector_transaction_id: Option<String>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not update this

let req = match payment_method_data {
domain::PaymentMethodData::Wallet(domain::WalletData::PaypalSdk(_)) => {
services::RequestBuilder::new()
.method(services::Method::Post)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this method Post if we are not sending a body?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using this flow to authorize the payment.
It should be Post Method - doc

_is_latency_header_enabled: Option<bool>,
) -> RouterResponse<Self> {
let papal_sdk_next_action =
paypal_sdk_next_steps_check(payment_data.get_payment_attempt().clone())?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should avoid doing this, the core orchestration layer should not handle connector specific logic. This refactor can be taken up separately

@@ -1774,6 +1809,7 @@ impl<F: Clone> TryFrom<PaymentAdditionalData<'_, F>> for types::PaymentsAuthoriz
charges,
merchant_order_reference_id,
integrity_object: None,
connector_transaction_id: payment_data.payment_attempt.connector_transaction_id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove this field from the request, we do not need connector_transaction_id to be present before the authorization has happened

.shipping_cost
.unwrap_or_default();
// amount here would include amount, surcharge_amount and shipping_cost
let amount = payment_data.payment_intent.amount + shipping_cost + surcharge_amount;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we not include order_tax_amount in case this was provided by the merchant in payments create? @sai-harsha-vardhan I think we should have the net amount struct for intent as well to handle these cases where some amount might be missed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the ideal way. We can introduce net_amount in intent domain models.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@swangi-kumari we should include order_tax amount here right?

.clone();
Ok(Self {
amount, //need to change after we move to connector module
currency: payment_data.currency,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: use this from payment intent. We do not want two source of truth for the same field. We plan on removing currency from payment data later in time

@@ -9,7 +9,8 @@ pub use api_models::payments::{
PaymentsApproveRequest, PaymentsCancelRequest, PaymentsCaptureRequest,
PaymentsCompleteAuthorizeRequest, PaymentsDynamicTaxCalculationRequest,
PaymentsDynamicTaxCalculationResponse, PaymentsExternalAuthenticationRequest,
PaymentsIncrementalAuthorizationRequest, PaymentsManualUpdateRequest, PaymentsRedirectRequest,
PaymentsIncrementalAuthorizationRequest, PaymentsManualUpdateRequest,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SanchithHegde what do you think about such re exports? This adds friction when a developer is trying to move his way to from where and how this struct has been imported. In my opinion we should always qualify specific imports from the parent module. This might take some time, but I think it will be better to move away from this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this was kept this way when we moved out API models to a separate crate, to reduce the number of changes elsewhere in code. But yeah, I think we should avoid such public re-exports.

Of course, this can be taken up in a separate PR.

.to_not_found_response(errors::ApiErrorResponse::PaymentNotFound)?;
payment_data.payment_attempt = updated_payment_attempt;
}
Err(_) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't ignore errors here, we should send back the relevant error code and error message back in the response

.shipping_cost
.unwrap_or_default();
// amount here would include amount, surcharge_amount and shipping_cost
let amount = payment_data.payment_intent.amount + shipping_cost + surcharge_amount;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the ideal way. We can introduce net_amount in intent domain models.

@@ -71,6 +71,18 @@ pub struct PaymentsAuthorizeData {
/// In case the connector supports only one reference id, Hyperswitch's Payment ID will be sent as reference.
pub merchant_order_reference_id: Option<String>,
pub integrity_object: Option<AuthoriseIntegrityObject>,
pub connector_transaction_id: Option<String>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not required anymore

@@ -579,6 +579,8 @@ Never share your secret api keys. Keep them guarded and secure.
api_models::payments::PaymentsDynamicTaxCalculationRequest,
api_models::payments::PaymentsDynamicTaxCalculationResponse,
api_models::payments::DisplayAmountOnSdk,
api_models::payments::PaymentsPostSessionTokensRequest,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are not adding the route, please remove the req / res structs from openapi_v2

event_builder: Option<&mut ConnectorEvent>,
res: Response,
) -> CustomResult<types::PaymentsPostSessionTokensRouterData, errors::ConnectorError> {
let response: paypal::PaypalRedirectResponse = res
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be PaypalOrdersResponse instead of PaypalRedirectResponse right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The response which get for PaypaSdk Create Order , matches the struct PaypalRedirectResponse.

@@ -955,7 +955,7 @@ fn create_paypal_sdk_session_token(
connector: connector.connector_name.to_string(),
session_token: paypal_sdk_data.data.client_id,
sdk_next_action: payment_types::SdkNextAction {
next_action: payment_types::NextActionCall::Confirm,
next_action: payment_types::NextActionCall::PostSessionTokens,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes can only go after SDK deployment right?
Due to this flow change, it would be backward incompatible. How would we handle staggered deployment here? @Narayanbhat166 @swangi-kumari

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this will go in sandbox after SDK sandbox deployment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this will go in sandbox after SDK sandbox deployment.

What's our strategy for production rollout of this change, would it be rolled out after SDK deployment on production as well?

.to_not_found_response(errors::ApiErrorResponse::ProfileNotFound {
id: profile_id.get_string_repr().to_owned(),
})?;
payment_attempt.payment_method = Some(request.payment_method);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is required for routing.

_key_store: &domain::MerchantKeyStore,
storage_scheme: enums::MerchantStorageScheme,
_locale: &Option<String>,
#[cfg(all(feature = "v1", feature = "dynamic_routing"))] _routable_connector: Vec<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These feature flags are redundant as we have v1 on top level

});
Ok(services::ApplicationResponse::JsonWithHeaders((
Self {
payment_id: payment_data.get_payment_attempt().payment_id.clone(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get payment_id from payment_intent here

@@ -498,6 +498,8 @@ pub enum Flow {
PaymentsManualUpdate,
/// Dynamic Tax Calcultion
SessionUpdateTaxCalculation,
/// Payments create order flow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Payments create order flow
/// Payments post session tokens flow

Comment on lines +4879 to +4882
/// The unique identifier for the payment
#[serde(skip_deserializing)]
#[schema(value_type = String)]
pub payment_id: id_type::PaymentId,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're not obtaining this from the request (not deserializing it), why are we including it in the request?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for ApiEventMetrics and ApiLocking

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Narayanbhat166 Should we consider having an "internal" struct for such purposes, or are we okay with this practice?

.url(&types::PaymentsPostSessionTokensType::get_url(
self, req, connectors,
)?)
.headers(types::PaymentsPostSessionTokensType::get_headers(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call attach_default_headers() as well?

.url(&types::PaymentsAuthorizeType::get_url(
self, req, connectors,
)?)
.headers(types::PaymentsAuthorizeType::get_headers(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

"Payment for invoice {}",
item.router_data.connector_request_reference_id
),
quantity: 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this is hardcoded?

Copy link
Contributor Author

@swangi-kumari swangi-kumari Oct 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially we had only 1 order, so we are treating whole order as single line item in order
We will discuss with with team, and refactor this in upcoming PRs.

@@ -955,7 +955,7 @@ fn create_paypal_sdk_session_token(
connector: connector.connector_name.to_string(),
session_token: paypal_sdk_data.data.client_id,
sdk_next_action: payment_types::SdkNextAction {
next_action: payment_types::NextActionCall::Confirm,
next_action: payment_types::NextActionCall::PostSessionTokens,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this will go in sandbox after SDK sandbox deployment.

What's our strategy for production rollout of this change, would it be rolled out after SDK deployment on production as well?

@@ -9,7 +9,8 @@ pub use api_models::payments::{
PaymentsApproveRequest, PaymentsCancelRequest, PaymentsCaptureRequest,
PaymentsCompleteAuthorizeRequest, PaymentsDynamicTaxCalculationRequest,
PaymentsDynamicTaxCalculationResponse, PaymentsExternalAuthenticationRequest,
PaymentsIncrementalAuthorizationRequest, PaymentsManualUpdateRequest, PaymentsRedirectRequest,
PaymentsIncrementalAuthorizationRequest, PaymentsManualUpdateRequest,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this was kept this way when we moved out API models to a separate crate, to reduce the number of changes elsewhere in code. But yeah, I think we should avoid such public re-exports.

Of course, this can be taken up in a separate PR.

@swangi-kumari
Copy link
Contributor Author

What's our strategy for production rollout of this change, would it be rolled out after SDK deployment on production as well?
Yes we will have SDK production deployment first.

@bernard-eugine bernard-eugine merged commit 53e82c3 into main Oct 15, 2024
22 of 25 checks passed
@bernard-eugine bernard-eugine deleted the create-order branch October 15, 2024 12:59
swangi-kumari added a commit that referenced this pull request Oct 15, 2024
Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com>
swangi-kumari added a commit that referenced this pull request Oct 17, 2024
Co-authored-by: hyperswitch-bot[bot] <148525504+hyperswitch-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-connector-integration Area: Connector integration A-core Area: Core flows C-feature Category: Feature request or enhancement C-refactor Category: Refactor M-api-contract-changes Metadata: This PR involves API contract changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants